Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

ci: introduce CI workflow to verify the CRD folder #483

Merged
merged 8 commits into from
Aug 9, 2023
Merged

ci: introduce CI workflow to verify the CRD folder #483

merged 8 commits into from
Aug 9, 2023

Conversation

tungbq
Copy link
Contributor

@tungbq tungbq commented Aug 3, 2023 •
edited
Loading

Special notes for your reviewer:

Close #480

Introduce CI workflow to verify the CRD folder, this will do following:

  • Set up Github Action environment: checkout code, setup kind, velero
  • Use velero and kubectl command to generate 2 YAML manifest files
  • Use diff command to check if these files are in synced or not
  • Archive the diff workspace for later debugging

This was tested on my forked repo:

The workflow also collects the artifact YAML manifest files and the diff results:

Currently I set the trigger event to run on workflow_dispatch only. Let me know if we need to cover other trigger events based.

Checklist

[Place an '[x]' (no spaces) in all applicable fields. Please remove unrelated fields.]

  • DCO signed
  • Chart Version bumped
  • Variables are documented in the values.yaml or README.md
  • Title of the PR starts with chart name (e.g. [velero])

@jenting
Copy link
Collaborator

jenting commented Aug 3, 2023 •
edited
Loading

Amazing work @tungbq 🤩

jenting
jenting requested changes Aug 3, 2023
View reviewed changes
.github/workflows/verify-manifest.yml Outdated Show resolved Hide resolved
@tungbq tungbq requested a review from jenting August 3, 2023 15:06
jenting
jenting reviewed Aug 3, 2023
View reviewed changes
.github/workflows/verify-manifest.yml Outdated Show resolved Hide resolved
.github/workflows/verify-manifest.yml Outdated Show resolved Hide resolved
.github/workflows/verify-manifest.yml Outdated Show resolved Hide resolved
@tungbq tungbq requested a review from jenting August 3, 2023 16:11
@jenting
Copy link
Collaborator

jenting commented Aug 4, 2023

@tungbq
CI failed. Could you plz check it?

@tungbq
Copy link
Contributor Author

tungbq commented Aug 4, 2023 •
edited
Loading

@tungbq CI failed. Could you plz check it?

The difference detected in the CI job due to this YAML block:
image

   status:
     acceptedNames:
       kind: ""
       plural: ""
     conditions: []
     storedVersions: []

These come from the final block of each crds YAML, e.g: https://github.com/vmware-tanzu/helm-charts/blob/main/charts/velero/crds/backuprepositories.yaml#L101-L106

I don't know the root cause for it now. Much appreciated if I could get some help here.
@jenting do you have any advice how to adjust or where could we correct it?

@jenting
Copy link
Collaborator

jenting commented Aug 4, 2023

@tungbq
We could use yq command to remove the status field because the status field is not required when apply the CRD to the kube-apiserver.

tungbq and others added 7 commits August 8, 2023 12:05
@tungbq
ci: introduce CI workflow to verify the CRD folder
2c41e2e 
Signed-off-by: Tung Bui <[email protected]>
@tungbq
ci: use verlero version from the charts value
ac4be5a 
Signed-off-by: Tung Bui <[email protected]>
@ishuar @tungbq
add the feature to allow relabeling in servicemonitor
e63451d 
Signed-off-by: ishuar <[email protected]>
Signed-off-by: Tung Bui <[email protected]>
@tungbq
ci: enable running on PR and remove archive artifacts step
beb75a1 
Signed-off-by: Tung Bui <[email protected]>
@jkroepke @tungbq
Add toggle to disable annotations
873c9d4 
Signed-off-by: Jan-Otto Kröpke <[email protected]>
Signed-off-by: Tung Bui <[email protected]>
@jkroepke @tungbq
bump version
3cce89f 
Signed-off-by: Jan-Otto Kröpke <[email protected]>
Signed-off-by: Tung Bui <[email protected]>
@tungbq
ci: filter non-required values element
9eeee06 
Signed-off-by: Tung Bui <[email protected]>
@tungbq
Copy link
Contributor Author

tungbq commented Aug 8, 2023

@tungbq We could use yq command to remove the status field because the status field is not required when apply the CRD to the kube-apiserver.

Thanks @jenting. Updated

jenting
jenting approved these changes Aug 8, 2023
View reviewed changes
Copy link
Collaborator

@jenting jenting left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Thank you @tungbq

@qiuming-best
Copy link
Collaborator

Thanks for your amazing work！ @tungbq

@qiuming-best qiuming-best merged commit 05c0831 into vmware-tanzu:main Aug 9, 2023
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qiuming-best qiuming-best qiuming-best approved these changes

@jenting jenting jenting approved these changes

Assignees

@tungbq tungbq

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Have A CI task to verify the CRD folder in sync
5 participants
@tungbq @jenting @qiuming-best @ishuar @jkroepke